Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Microsoft integration] getFullMessageList #9544

Merged
merged 8 commits into from
Jan 13, 2025
Merged

Conversation

guillim
Copy link
Contributor

@guillim guillim commented Jan 10, 2025

Creation of the GmailGetMessageListService
Implementation of the driver to MS Graph API getFullMessageList

@guillim guillim self-assigned this Jan 10, 2025
Copy link

github-actions bot commented Jan 10, 2025

Fails
🚫

node failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-339fc66a.json

Generated by 🚫 dangerJS against e355966

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Implemented Microsoft Graph API integration for message fetching, adding support for retrieving email messages from Microsoft 365 alongside the existing Gmail integration.

  • Fixed incorrect MESSAGING_MICROSOFT_USERS_MESSAGES_LIST_MAX_RESULT value in /packages/twenty-server/src/modules/messaging/message-import-manager/drivers/microsoft/services/microsoft-get-message-list.service.ts (set to 1 instead of 1000)
  • Missing error handling for Microsoft Graph API calls in microsoft-get-message-list.service.ts
  • Using beta API version instead of stable in Microsoft Graph API calls
  • Incomplete getPartialMessageList implementation for Microsoft provider returns empty values
  • Incorrect redirect URIs in documentation referencing Google endpoints instead of Microsoft ones

7 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +34 to +40
const response: PageCollection = await microsoftClient
.api(syncCursor || '/me/mailfolders/inbox/messages/delta?$select=id')
.version('beta')
.headers({
Prefer: `odata.maxpagesize=${MESSAGING_MICROSOFT_USERS_MESSAGES_LIST_MAX_RESULT}`,
})
.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: No try-catch block around API call that could fail due to network issues or invalid tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only interesting thing we might talk out loud

Copy link
Contributor Author

@guillim guillim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charlesBochet charlesBochet merged commit 34ee64a into main Jan 13, 2025
34 checks passed
@charlesBochet charlesBochet deleted the outlook-integration branch January 13, 2025 09:13
@guillim guillim restored the outlook-integration branch January 13, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants